Skip to content
This repository has been archived by the owner on Feb 9, 2024. It is now read-only.

(5.5) Fix issue with chown when only uid is specified #1789

Merged
merged 2 commits into from
Jun 25, 2020

Conversation

r0mant
Copy link
Contributor

@r0mant r0mant commented Jun 24, 2020

Description

This PR addresses the issue with upgrade failing to update permissions of a new volume if only uid is set due to an incorrect chown command (see linked ticket for more details).

I have also updated a piece of code responsible to disabling remote access (e.g. to a wizard) to tolerate "not found" errors for authorities to ensure reentrability - otherwise if a later call (e.g. deleting a trusted cluster) fails due to etcd error, the authorities won't be there and it'll fail with not found.

Type of change

  • Bug fix (non-breaking change which fixes an issue)

Linked tickets and other PRs

TODOs

  • Self-review the change
  • Write tests
  • Perform manual testing
  • Address review feedback

Testing done

Install a cluster, then upgrade to a new version that has this volume:

 nodeProfiles:
   - name: worker
     description: Worker
     requirements:
       volumes:
         - name: data
           path: /var/data
           targetPath: /data
           uid: 3000

Before the change, upgrade fails with:

User Message: failed to execute phase "/bootstrap"
	failed to run {[chown 3000: /var/data] setting ownership of /var/data to 3000: false}:
		exit status 1] handler:upgrade utils/logging.go:103

After the change, the upgrade passes and volume is chown'd:

ubuntu@node-1:~/upgrade$ ls -l /var | grep data
drwxr-xr-x  3 3000 root   4096 May 28 00:05 data

@r0mant r0mant requested a review from a team June 24, 2020 21:38
@r0mant r0mant self-assigned this Jun 24, 2020
@r0mant r0mant mentioned this pull request Jun 25, 2020
14 tasks
@r0mant r0mant merged commit 63ef1fe into version/5.5.x Jun 25, 2020
@r0mant r0mant deleted the roman/5.5/uid branch June 25, 2020 15:19
r0mant added a commit that referenced this pull request Jun 26, 2020
* Fix issue with chown when only uid is specified

* Ignore not found authority errors when disabling access
r0mant added a commit that referenced this pull request Jun 26, 2020
* Fix issue with chown when only uid is specified

* Ignore not found authority errors when disabling access
r0mant added a commit that referenced this pull request Jun 26, 2020
* Fix issue with chown when only uid is specified

* Ignore not found authority errors when disabling access
a-palchikov pushed a commit that referenced this pull request Jun 26, 2020
* Fix issue with chown when only uid is specified

* Ignore not found authority errors when disabling access
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants